[SM6.10] VEC9 TriangleObjectPositions / CHECK-pass validation tests#7831
Conversation
|
Hey Simon! We've actually been talking about DXIL op stuff internally that this PR is going to fall under. Specially we want a better system to mark ops as experimental before they are upgraded to stable! I have a proposal up (microsoft/hlsl-specs#698) but due to scheduling conflicts we aren't going to be able to make a final decision until ~Nov 4th. Since the decision there has impacts on this PR we are holding off on the review for just a little bit. I wanted to give you a heads up so you weren't surprised by the silence from our side |
|
The proposal is up here, once the implementation lands the ops here will need to be renumbered. I suspect the implementation will land quite quickly |
|
Spec has been merged! I'm on vacation the next few weeks but @tex3d should be uploading the infrastructure change soon. Then this can be rebased on that |
I'm trying to get a second review on this: #7947. Once merged, you can update the branch and move the intrinsic definitions in hctdb.py into experimental and update any references to the opcode numbers that changed. |
|
I believe we'll also want to reserve high-level intrinsic IDs for these ASAP. It might be good to do this in the same PR, but a separate PR for that would be ok too. That means updates to This could serve as a good example for how to reserve HLSL intrinsics and DXIL opcodes for a feature before it's implemented, using the new experimental DXIL op table. |
|
I've created a PR to reserve HL and DXIL ops for these operations here: #7995. So, you should be able to re-apply just the changes necessary on top of main when that PR merges. |
SM6.10 tracking bug: microsoft#7824
simoll
changed the title
simoll
force-pushed
the
sm610_dxil_opcodes
branch
from
b3a793f to
63a5b8b
Compare
tex3d
left a comment
tex3d
left a comment
There was a problem hiding this comment.
I think this looks good, with just one concern:
The signed form of the i32 opcode is the one llvm will print and the one that will be needed in CHECK lines of tests. I think it would be best to stick to the signed form for input IR as well, so that each opcode doesn't have two textural forms to search/replace when updating ops in tests for released versions.
tex3d
left a comment
tex3d
left a comment
There was a problem hiding this comment.
I added suggested change comments to make updating the opcodes to canonical form easier.
Co-authored-by: Tex Riddell <texr@microsoft.com>
Co-authored-by: Tex Riddell <texr@microsoft.com>
Co-authored-by: Tex Riddell <texr@microsoft.com>
Co-authored-by: Tex Riddell <texr@microsoft.com>
|
@tex3d Please re-review. All tests use signed opcodes in the input IR now |
tex3d
left a comment
tex3d
left a comment
There was a problem hiding this comment.
I think the change is good, but the tests need to be moved if you want them to run, and they need a REQUIRES line to prevent them from running against down-level released DXIL validators.
|
Thanks . I've moved the tests to LitDXILValidation and add the REQUIRES: dxil-1-10 clause. |
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
5 participants
SM6.10 tracking bug: #7824